Repository navigation
Fix protected-main auto-release token and contracts namespaces - #366
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
@codex review /gemini review @coderabbitai full review |
|
To use Codex here, create a Codex account and connect to github. |
|
Warning Review limit reached
More reviews will be available in 21 minutes. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThe PR adds ChangesContracts namespace recovery
Auto-release protected push gating
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Action performedFull review finished. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request resolves two main issues: it updates the contracts command lookup to handle package namespace roots by throwing a descriptive command_namespace error with child command suggestions, and it remediates the Auto Release pipeline block on protected branches by utilizing a RELEASE_PAT secret. Additionally, it updates @sentry/node to ^10.61.0 and documents the release requirements. Regarding the feedback, the reviewer correctly identified that nesting the new namespace root contracts test case inside an unrelated test block reduces modularity, and suggested splitting it into its own dedicated it block for better clarity.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| it("rejects unknown action and command filters", async () => { | ||
| await withTempPmPath(async (context) => { | ||
| await writeTestExtension({ | ||
| root: context.pmPath, | ||
| placement: "projectRoot", | ||
| directory: "changelog-namespace-contracts", | ||
| manifest: { | ||
| name: "changelog-namespace-contracts", | ||
| version: "1.0.0", | ||
| entry: "./index.mjs", | ||
| capabilities: ["commands", "schema"], | ||
| }, | ||
| entryFilename: "index.mjs", | ||
| entrySource: [ | ||
| "export default {", | ||
| " activate(api) {", | ||
| " api.registerCommand({", | ||
| " name: 'changelog generate',", | ||
| " action: 'changelog-generate',", | ||
| " flags: [{ long: '--output', value_name: 'file', value_type: 'string' }],", | ||
| " run: () => ({ ok: true }),", | ||
| " });", | ||
| " api.registerCommand({", | ||
| " name: 'changelog export',", | ||
| " action: 'changelog-export',", | ||
| " flags: [{ long: '--format', value_name: 'format', value_type: 'string' }],", | ||
| " run: () => ({ ok: true }),", | ||
| " });", | ||
| " },", | ||
| "};", | ||
| "", | ||
| ].join("\n"), | ||
| }); | ||
|
|
||
| await expect( | ||
| runContracts( | ||
| { command: "changelog", flagsOnly: true }, | ||
| { ...GLOBAL_OPTIONS, path: context.pmPath }, | ||
| ), | ||
| ).rejects.toMatchObject<PmCliError>({ | ||
| message: | ||
| 'Command "changelog" is a command namespace. Choose a concrete child command: changelog export, changelog generate.', | ||
| exitCode: EXIT_CODE.USAGE, | ||
| context: expect.objectContaining({ | ||
| code: "command_namespace", | ||
| required: "Use a concrete child command path listed under this namespace.", | ||
| why: expect.stringContaining("command flag contracts belong to executable child commands"), | ||
| examples: expect.arrayContaining([ | ||
| 'pm contracts --command "changelog export" --flags-only --json', | ||
| 'pm contracts --command "changelog generate" --flags-only --json', | ||
| ]), | ||
| recovery: expect.objectContaining({ | ||
| suggested_retry: 'pm contracts --command "changelog export" --flags-only --json', | ||
| fallback_candidates: expect.arrayContaining([ | ||
| { | ||
| source: "command_namespace", | ||
| command: 'pm contracts --command "changelog generate" --flags-only --json', | ||
| reason: "Child command under changelog", | ||
| }, | ||
| ]), | ||
| }), | ||
| }), | ||
| }); | ||
|
|
||
| const childResult = await runContracts( | ||
| { command: "changelog generate", flagsOnly: true }, | ||
| { ...GLOBAL_OPTIONS, path: context.pmPath }, | ||
| ); | ||
| expect(childResult.command_flags).toEqual([ | ||
| expect.objectContaining({ | ||
| command: "changelog generate", | ||
| provider: "extension", | ||
| flags: [expect.objectContaining({ flag: "--output" })], | ||
| }), | ||
| ]); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Nesting the new namespace root contracts test case inside the existing it("rejects unknown action and command filters") block reduces test modularity and makes test reports less clear. Splitting it into its own dedicated it block improves maintainability and ensures the test suite's output explicitly documents this specific behavior.
it("rejects command namespace roots and suggests concrete child commands", async () => {
await withTempPmPath(async (context) => {
await writeTestExtension({
root: context.pmPath,
placement: "projectRoot",
directory: "changelog-namespace-contracts",
manifest: {
name: "changelog-namespace-contracts",
version: "1.0.0",
entry: "./index.mjs",
capabilities: ["commands", "schema"],
},
entryFilename: "index.mjs",
entrySource: [
"export default {",
" activate(api) {",
" api.registerCommand({",
" name: 'changelog generate',",
" action: 'changelog-generate',",
" flags: [{ long: '--output', value_name: 'file', value_type: 'string' }],",
" run: () => ({ ok: true }),",
" });",
" api.registerCommand({",
" name: 'changelog export',",
" action: 'changelog-export',",
" flags: [{ long: '--format', value_name: 'format', value_type: 'string' }],",
" run: () => ({ ok: true }),",
" });",
" },",
"};",
"",
].join("\n"),
});
await expect(
runContracts(
{ command: "changelog", flagsOnly: true },
{ ...GLOBAL_OPTIONS, path: context.pmPath },
),
).rejects.toMatchObject<PmCliError>({
message:
'Command "changelog" is a command namespace. Choose a concrete child command: changelog export, changelog generate.',
exitCode: EXIT_CODE.USAGE,
context: expect.objectContaining({
code: "command_namespace",
required: "Use a concrete child command path listed under this namespace.",
why: expect.stringContaining("command flag contracts belong to executable child commands"),
examples: expect.arrayContaining([
'pm contracts --command "changelog export" --flags-only --json',
'pm contracts --command "changelog generate" --flags-only --json',
]),
recovery: expect.objectContaining({
suggested_retry: 'pm contracts --command "changelog export" --flags-only --json',
fallback_candidates: expect.arrayContaining([
{
source: "command_namespace",
command: 'pm contracts --command "changelog generate" --flags-only --json',
reason: "Child command under changelog",
},
]),
}),
}),
});
const childResult = await runContracts(
{ command: "changelog generate", flagsOnly: true },
{ ...GLOBAL_OPTIONS, path: context.pmPath },
);
expect(childResult.command_flags).toEqual([
expect.objectContaining({
command: "changelog generate",
provider: "extension",
flags: [expect.objectContaining({ flag: "--output" })],
}),
]);
});
});
it("rejects unknown action and command filters", async () => {
|
I have reviewed the changes regarding the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/auto-release.yml (1)
54-63: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftElevated
RELEASE_PATis persisted to the runner's git config for all later steps.
actions/checkoutdefaults topersist-credentials: true, so the maintainerRELEASE_PAT(which carriescontents:write+ branch-protection bypass) is written into.git/configand remains readable by every subsequent step in the job — includingpnpm install --frozen-lockfile(line 98), which can execute dependency lifecycle/build scripts. A compromised or malicious transitive dependency could exfiltrate this high-privilege token.Note that the naive fix (
persist-credentials: false) is not viable here becauserun-release-pipeline.mjsrelies on the persisted credentials to push the version commit/tag tomain. A scoped approach — e.g. checking out withpersist-credentials: falseand injecting the PAT only into the push step (via a per-stepextraheader/remote URL) — would limit the exposure window to the push instead of the whole job. Please confirm the current exposure is acceptable given the token's bypass rights.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/auto-release.yml around lines 54 - 63, The Checkout step is persisting the high-privilege RELEASE_PAT into git credentials for the entire job, exposing it to later steps like dependency installs. Update the workflow so actions/checkout does not keep credentials globally, and instead scope the maintainer token only to the release push path used by run-release-pipeline.mjs, such as injecting it just for the version/tag push step or equivalent per-step remote auth. Keep the fix localized to the Checkout and push flow so the token is only available when needed.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/auto-release.yml:
- Around line 54-63: The Checkout step is persisting the high-privilege
RELEASE_PAT into git credentials for the entire job, exposing it to later steps
like dependency installs. Update the workflow so actions/checkout does not keep
credentials globally, and instead scope the maintainer token only to the release
push path used by run-release-pipeline.mjs, such as injecting it just for the
version/tag push step or equivalent per-step remote auth. Keep the fix localized
to the Checkout and push flow so the token is only available when needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3b1ae1bf-c9d2-4f02-b785-813821c43145
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
.agents/pm/history/pm-9gxi.jsonl.agents/pm/history/pm-y1o4.jsonl.agents/pm/issues/pm-9gxi.toon.agents/pm/issues/pm-y1o4.toon.github/workflows/auto-release.ymlCHANGELOG.mddocs/RELEASING.mdpackage.jsonsrc/cli/commands/contracts.tstests/integration/ci-workflow-contract.spec.tstests/integration/release-automation-contract.spec.tstests/unit/commands/contracts-command.spec.ts
9abe50a to
5142e7a
Compare
|
Review feedback addressed in 5142e7a: split the package namespace-root contracts regression into its own dedicated test case and reran
RUN v4.1.9 /home/steve/GITHUB_RELEASE/pm-cli Test Files 1 passed (1) @codex review /gemini review @coderabbitai full review |
|
To use Codex here, create a Codex account and connect to github. |
|
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! ✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request updates the auto-release workflow to use a maintainer RELEASE_PAT to bypass branch protection rules on main when pushing version-bump commits, adding a fail-fast preflight check and updating relevant documentation and integration tests. It also updates @sentry/node to ^10.61.0. Additionally, it resolves an issue where looking up package namespace roots in the contracts command returned an unknown command error; it now throws a specific command_namespace error suggesting concrete child commands, backed by new unit tests. Review feedback suggests consolidating duplicate test assertions for the auto-release.yml workflow across release-automation-contract.spec.ts and ci-workflow-contract.spec.ts to improve maintainability.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| expect(workflow).toContain("token: ${{ secrets.RELEASE_PAT || github.token }}"); | ||
| expect(workflow).toContain("RELEASE_PAT_CONFIGURED: ${{ secrets.RELEASE_PAT != '' }}"); | ||
| expect(workflow).toContain("RELEASE_PAT is required before Auto Release can push"); | ||
| expect(workflow).toContain("gh workflow run release.yml --ref main -f tag=\"${NEW_TAG}\""); | ||
| expect(workflow).toContain("gh run watch \"${RELEASE_RUN_ID}\" --compact --exit-status --interval 30"); | ||
| expect(workflow).toContain("SENTRY_AUTH_TOKEN"); | ||
| expect(workflow).toContain("SENTRY_PERSONAL_ADMIN_TOKEN"); | ||
| expect(workflow).toContain("Common causes:"); |
There was a problem hiding this comment.
There appears to be some test duplication for the auto-release.yml workflow. This file and tests/integration/ci-workflow-contract.spec.ts both contain a test case that reads and asserts on the contents of auto-release.yml.
Specifically, the test it("keeps auto-release workflow aligned with one-per-day and manual override controls", ...) in this file and it("keeps auto-release workflow aligned with one-per-day and manual override policy", ...) in ci-workflow-contract.spec.ts seem to have overlapping responsibilities.
To improve maintainability and avoid needing to update two places for a single workflow change, consider consolidating these tests into a single, comprehensive test case for auto-release.yml in one of the files.
5142e7a to
876541c
Compare
|
Review loop restarted for current head 876541c. Changes since the prior review:
Local verification on this head:
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
Re-running a full review for the current head ✏️ Learnings added
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request addresses two main issues: it resolves a protected-branch push block in the Auto Release pipeline by scoping and injecting a maintainer RELEASE_PAT via a Git extraheader, and it improves the contracts command to handle package namespace roots by throwing a descriptive error and suggesting concrete child commands. Additionally, @sentry/node is updated to 10.61.0, and duplicate workflow assertions are cleaned up. The review feedback suggests standardizing the HTTP header and authentication scheme casing to Authorization: Basic in both the release pipeline script and its corresponding unit tests to ensure maximum compatibility.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (!token) { | ||
| return gitOptions; | ||
| } | ||
| const authHeader = `AUTHORIZATION: basic ${Buffer.from(`x-access-token:${token}`, "utf8").toString("base64")}`; |
There was a problem hiding this comment.
Standardizing the HTTP header name and authentication scheme casing to Authorization: Basic is highly recommended. While HTTP headers and auth schemes are technically case-insensitive per RFC 7235, some strict proxies, firewalls, or older Git/curl builds may fail to parse non-standard casing like AUTHORIZATION: basic. Using the standard casing ensures maximum compatibility across all network environments.
| const authHeader = `AUTHORIZATION: basic ${Buffer.from(`x-access-token:${token}`, "utf8").toString("base64")}`; | |
| const authHeader = `Authorization: Basic ${Buffer.from(`x-access-token:${token}`, "utf8").toString("base64")}`; |
| const expectedHeader = `AUTHORIZATION: basic ${Buffer.from( | ||
| "x-access-token:release-token", | ||
| "utf8", | ||
| ).toString("base64")}`; |
There was a problem hiding this comment.
Update the test assertion to match the standardized Authorization: Basic header casing.
| const expectedHeader = `AUTHORIZATION: basic ${Buffer.from( | |
| "x-access-token:release-token", | |
| "utf8", | |
| ).toString("base64")}`; | |
| const expectedHeader = `Authorization: Basic ${Buffer.from( | |
| "x-access-token:release-token", | |
| "utf8", | |
| ).toString("base64")}`; |
876541c to
960a21b
Compare
|
Review loop restarted for current head 960a21b. Fresh change after Gemini feedback:
Local verification on this head:
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
I’m re-running a full review now with attention to that release-push credential handling update and the surrounding release/contracts coverage. ✏️ Learnings added
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request introduces scoped git push authentication using a maintainer RELEASE_PAT to prevent Auto Release blockages on protected branches, alongside corresponding workflow, script, test, and documentation updates. Additionally, it enhances the contracts command to gracefully handle package namespace roots by suggesting concrete child commands instead of throwing an unknown command error. Feedback is provided to dynamically append Git configuration variables in withReleasePushCredentials to avoid overwriting existing environment configurations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| export function withReleasePushCredentials(gitOptions = {}, token = releasePushToken) { | ||
| if (!token) { | ||
| return gitOptions; | ||
| } | ||
| const authHeader = `Authorization: Basic ${Buffer.from(`x-access-token:${token}`, "utf8").toString("base64")}`; | ||
| return { | ||
| ...gitOptions, | ||
| env: { | ||
| ...(gitOptions.env ?? {}), | ||
| GIT_CONFIG_COUNT: "1", | ||
| GIT_CONFIG_KEY_0: "http.https://github.com/.extraheader", | ||
| GIT_CONFIG_VALUE_0: authHeader, | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Hardcoding GIT_CONFIG_COUNT: "1" and GIT_CONFIG_KEY_0 / GIT_CONFIG_VALUE_0 can silently overwrite or discard any existing Git configuration variables passed in gitOptions.env.
To make this robust and prevent potential configuration conflicts or loss in future refactorings, dynamically read the existing GIT_CONFIG_COUNT (if any) and append the new authorization header config at the next available index.
| export function withReleasePushCredentials(gitOptions = {}, token = releasePushToken) { | |
| if (!token) { | |
| return gitOptions; | |
| } | |
| const authHeader = `Authorization: Basic ${Buffer.from(`x-access-token:${token}`, "utf8").toString("base64")}`; | |
| return { | |
| ...gitOptions, | |
| env: { | |
| ...(gitOptions.env ?? {}), | |
| GIT_CONFIG_COUNT: "1", | |
| GIT_CONFIG_KEY_0: "http.https://github.com/.extraheader", | |
| GIT_CONFIG_VALUE_0: authHeader, | |
| }, | |
| }; | |
| } | |
| export function withReleasePushCredentials(gitOptions = {}, token = releasePushToken) { | |
| if (!token) { | |
| return gitOptions; | |
| } | |
| const authHeader = 'Authorization: Basic ' + Buffer.from('x-access-token:' + token, 'utf8').toString('base64'); | |
| const env = { ...(gitOptions.env ?? {}) }; | |
| const count = parseInt(env.GIT_CONFIG_COUNT ?? '0', 10); | |
| env['GIT_CONFIG_KEY_' + count] = 'http.https://github.com/.extraheader'; | |
| env['GIT_CONFIG_VALUE_' + count] = authHeader; | |
| env.GIT_CONFIG_COUNT = String(count + 1); | |
| return { | |
| ...gitOptions, | |
| env, | |
| }; | |
| } |
960a21b to
6f800bf
Compare
|
Review loop restarted for current head 6f800bf. Fresh change after Gemini feedback:
Local verification on this head:
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
Code Review
This pull request addresses two main issues: it resolves a block in the Auto Release pipeline where the bot could not push version commits to the protected main branch, and it improves the contracts command lookup to handle package namespace roots. For the release pipeline, the workflow is updated to use a maintainer RELEASE_PAT (passed as RELEASE_PUSH_TOKEN), which is securely scoped only to the git push process via a temporary Git extraheader and deleted from process.env to prevent inheritance by child processes. For the contracts command, looking up a namespace root (e.g., changelog) now throws a clear command_namespace error suggesting concrete executable child commands instead of an unknown_command error. Additionally, @sentry/node has been updated to ^10.61.0 along with other lockfile dependencies, and comprehensive integration and unit tests have been added to verify these changes. No reviewer comments were provided, so there is no additional feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Re-running a full review now with attention to that Git config env handling change and the surrounding release-token isolation behavior. ✏️ Learnings added
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/auto-release.yml (1)
113-128: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate the same token value the release script will use.
Line 124 only checks
RELEASE_PAT_CONFIGURED, butrun-release-pipeline.mjstrimsRELEASE_PUSH_TOKENand skips credential injection when it becomes empty. A whitespace-onlyRELEASE_PATcan pass this preflight, then fail later atgit pushwithout the actionable guard.Proposed guard alignment
- if [[ "${INPUT_PUSH}" == "true" && "${INPUT_DRY_RUN}" != "true" && "${RELEASE_PAT_CONFIGURED}" != "true" ]]; then + if [[ "${INPUT_PUSH}" == "true" && "${INPUT_DRY_RUN}" != "true" && ( "${RELEASE_PAT_CONFIGURED}" != "true" || -z "${RELEASE_PUSH_TOKEN//[[:space:]]/}" ) ]]; then echo "RELEASE_PAT is required before Auto Release can push the checked version commit/tag to protected main." echo "Create a release environment secret from a maintainer token with contents:write and branch-protection bypass rights." exit 1 fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/auto-release.yml around lines 113 - 128, The preflight check in the auto-release workflow is validating only RELEASE_PAT_CONFIGURED, but the release pipeline uses RELEASE_PUSH_TOKEN after trimming, so whitespace-only or empty tokens can still slip through and fail later. Update the guard in the workflow run step to validate the same effective token value that run-release-pipeline.mjs will use, and reject any blank/trimmed-empty RELEASE_PAT before allowing push=true. Keep the existing INPUT_PUSH and INPUT_DRY_RUN logic, but align the token presence check with RELEASE_PUSH_TOKEN so the early failure matches the later credential injection behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/auto-release.yml:
- Around line 113-128: The preflight check in the auto-release workflow is
validating only RELEASE_PAT_CONFIGURED, but the release pipeline uses
RELEASE_PUSH_TOKEN after trimming, so whitespace-only or empty tokens can still
slip through and fail later. Update the guard in the workflow run step to
validate the same effective token value that run-release-pipeline.mjs will use,
and reject any blank/trimmed-empty RELEASE_PAT before allowing push=true. Keep
the existing INPUT_PUSH and INPUT_DRY_RUN logic, but align the token presence
check with RELEASE_PUSH_TOKEN so the early failure matches the later credential
injection behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6f2dda70-55b4-4b84-b75c-961446d0bd79
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
.agents/pm/history/pm-9gxi.jsonl.agents/pm/history/pm-y1o4.jsonl.agents/pm/issues/pm-9gxi.toon.agents/pm/issues/pm-y1o4.toon.github/workflows/auto-release.ymlCHANGELOG.mddocs/RELEASING.mdpackage.jsonscripts/release/run-release-pipeline.mjssrc/cli/commands/contracts.tstests/integration/ci-workflow-contract.spec.tstests/integration/release-automation-contract.spec.tstests/unit/commands/contracts-command.spec.tstests/unit/scripts/release/run-release-pipeline.spec.ts
💤 Files with no reviewable changes (1)
- tests/integration/release-automation-contract.spec.ts
6f800bf to
ae27a82
Compare
|
To use Codex here, create a Codex account and connect to github. |
|
Tip For best results, initiate chat on the files or code changes.
Re-running a full review now with attention to that large-extension namespace-root behavior and the surrounding contracts UX/tests. [full_review] ✏️ Learnings added
|
There was a problem hiding this comment.
Code Review
This pull request secures the auto-release pipeline by scoping the RELEASE_PUSH_TOKEN strictly to git push operations via a temporary git extraheader, and updates @sentry/node to 10.61.0. It also enhances the contracts command to gracefully handle package namespace roots by returning token-efficient, capped child command suggestions instead of an unknown command error. Feedback on the changes suggests adding a defensive check in withReleasePushCredentials to handle cases where gitOptions is explicitly passed as null.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| export function withReleasePushCredentials(gitOptions = {}, token = releasePushToken) { | ||
| if (!token) { | ||
| return gitOptions; | ||
| } | ||
| const baseEnv = gitOptions.env ?? {}; | ||
| const existingGitConfigCount = Number.parseInt(baseEnv.GIT_CONFIG_COUNT ?? "0", 10); | ||
| const gitConfigIndex = Number.isInteger(existingGitConfigCount) && existingGitConfigCount >= 0 | ||
| ? existingGitConfigCount | ||
| : 0; | ||
| const authHeader = `Authorization: Basic ${Buffer.from(`x-access-token:${token}`, "utf8").toString("base64")}`; | ||
| return { | ||
| ...gitOptions, | ||
| env: { | ||
| ...baseEnv, | ||
| GIT_CONFIG_COUNT: String(gitConfigIndex + 1), | ||
| [`GIT_CONFIG_KEY_${gitConfigIndex}`]: "http.https://github.com/.extraheader", | ||
| [`GIT_CONFIG_VALUE_${gitConfigIndex}`]: authHeader, | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
To adhere to defensive programming practices, add a guard for cases where gitOptions might be explicitly passed as null. This prevents potential TypeError when accessing gitOptions.env.
export function withReleasePushCredentials(gitOptions = {}, token = releasePushToken) {
if (!token) {
return gitOptions;
}
const options = gitOptions ?? {};
const baseEnv = options.env ?? {};
const existingGitConfigCount = Number.parseInt(baseEnv.GIT_CONFIG_COUNT ?? "0", 10);
const gitConfigIndex = Number.isInteger(existingGitConfigCount) && existingGitConfigCount >= 0
? existingGitConfigCount
: 0;
const authHeader = "Authorization: Basic " + Buffer.from("x-access-token:" + token, "utf8").toString("base64");
return {
...options,
env: {
...baseEnv,
GIT_CONFIG_COUNT: String(gitConfigIndex + 1),
["GIT_CONFIG_KEY_" + gitConfigIndex]: "http.https://github.com/.extraheader",
["GIT_CONFIG_VALUE_" + gitConfigIndex]: authHeader,
},
};
}
Scope protected-main Auto Release credentials to git push by passing RELEASE_PAT as RELEASE_PUSH_TOKEN, clearing it from process.env, and injecting a temporary Git extraheader only for push attempts. Keep checkout persist-credentials=false and fail fast when production push runs lack a nonblank maintainer release token. Improve pm contracts package namespace UX by returning token-efficient command_namespace recovery for namespace roots such as changelog while preserving executable child command flag contracts. Add regression coverage for namespace roots and bounded fallback candidates. Refresh @sentry/node to 10.61.0, keep the generated changelog aligned through pm-changelog, and record PM evidence for pm-9gxi and pm-y1o4 including the remaining external RELEASE_PAT setup state. Verification: focused release/contract tests, full coverage at 100/100/100/100, static/docstring gate at 100%, contracts snapshot check, pm health, changelog check, and manual temp-directory pm smoke.
da71620 to
735e8dc
Compare
|
Review feedback addressed in 735e8dc. Changes since the prior review:
Local verification on this head:
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
I’m kicking off a fresh full review now with attention to that release-push credential edge case and the surrounding token-scoping behavior. ✏️ Learnings added
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request addresses two main issues: it resolves a blocker in the Auto Release pipeline where the bot could not push version commits to the protected main branch, and it improves the contracts command to gracefully handle package namespace roots. The release pipeline was updated to use a scoped RELEASE_PUSH_TOKEN (configured via RELEASE_PAT) injected dynamically as a Git extraheader during push operations, ensuring credentials are not persisted or leaked to child processes. Additionally, the contracts command now detects when a namespace root is queried and provides token-efficient suggestions for concrete child commands, capped to prevent excessive payload sizes. Relevant documentation, dependency updates for @sentry/node, and comprehensive integration and unit tests were also added. No reviewer comments were provided for evaluation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Release the pm-9gxi working claim after PR #366 merged while leaving the item blocked on the external RELEASE_PAT repository secret setup.
Confirmed post-#366 that the release environment still lacks RELEASE_PAT, so the scheduled auto-release fails fast until a maintainer adds the secret. The push=false dry-run confirmed the rest of the pipeline is healthy (computes v2026.6.25). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXrEXbgSQDzLazicMTH2Tn
Summary
RELEASE_PATwith a production push preflight so protected-main release pushes fail early with actionable guidance instead of GH006 after all gates.RELEASE_PATand improve blocked-release alert recovery text.pm contracts --command <namespace>UX for package command roots by returningcommand_namespacechild-command guidance, while preserving child command flag contracts.@sentry/nodeto 10.61.0 and regenerateCHANGELOG.mdviapm-changelog2026.6.20.PM tracking
pm-y1o4closed and released with tests/docs/evidence.pm-9gxiremains blocked, not closed: code is implemented and verified, but the release environment still needs a maintainer-providedRELEASE_PATsecret.Verification
node scripts/run-tests.mjs test -- tests/unit/commands/contracts-command.spec.ts tests/integration/release-automation-contract.spec.ts tests/integration/ci-workflow-contract.spec.ts(57 tests)node scripts/run-tests.mjs coverage=> 100/100/100/100node scripts/release/static-quality-gate.mjs --json=> docstrings 237/237 source, 1840/1840 exportednode scripts/release/run-gates.mjs --json --telemetry-mode offnode scripts/release/sentry-telemetry-gate.mjs --json --telemetry-mode best-effort=> Sentry critical=0 high=0, telemetry finish error rate 0.78%, missing error-code rows=0pnpm contracts:checkpnpm changelog:pm:checkpnpm audit --audit-level lowpnpm security:scan/tmp/pm-cli-manual-smoke-NS6WJBusing sandboxedPM_PATHandPM_GLOBAL_PATH: init/create/claim/update/files/docs/tests/comment/list/validate/health/close/release